Change Json.NET to System.Text.Json #3758

tdykstra · 2020-01-10T03:56:31Z

scottaddie · 2020-01-10T15:01:34Z

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

@@ -16,7 +16,7 @@
  <Interfaces />
  <Docs>
    <summary>
-      <see href="https://www.newtonsoft.com/json">Json.NET</see> should be used for serialization and deserialization. Provides serialization and deserialization functionality for AJAX-enabled applications.</summary>
+      The APIs in the <see cref="N:System.Text.Json" /> namespace should be used for serialization and deserialization. Provides serialization and deserialization functionality for AJAX-enabled applications.</summary>


Isn't this statement true ONLY for .NET Framework 4.6.1 or later?

Use Json.NET instead of this class

Yes, I recall switching from JavaScriptSerializer to JSON.NET years ago for performance reasons. At one point, there was messaging from Microsoft that completely discouraged the use of JavaScriptSerializer.

@ahsonkhan In this doc should we say use Newtonsoft.json for earlier than 4.6.1 and System.Text.Json for 4.6.1 and later?

@layomia @jozkee @steveharter can you please help answer this question?

@steveharter @layomia @jozkee ping. What is the correct sentence to use here?

Yes, System.Text.Json targets netstandard2.0 which makes it compatible only with .NET Framework 4.6.1 or later.

should we say use Newtonsoft.json for earlier than 4.6.1 and System.Text.Json for 4.6.1 and later

That seems to be the most accurate. Why is that advice on the summary and not as a warning in the JavaScriptSerializer page?

Why is that advice on the summary and not as a warning in the JavaScriptSerializer page?

No way to know why it's there or who put it there -- it's already there in the earliest commit in the repo.

I agree it's out of place where it is, I'll move it to Remarks.

I'll move it to Remarks.

If we only have it in remarks, it won't show up in IntelliSense for folks using IDEs like VS. We may want to keep the note in the summary as well as the remarks.

ahsonkhan · 2020-03-25T01:13:11Z

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

- The <xref:System.Web.Script.Serialization.JavaScriptSerializer> class is used internally by the asynchronous communication layer to serialize and deserialize the data that is passed between the browser and the Web server. You cannot access that instance of the serializer. However, this class exposes a public API. Therefore, you can use the class when you want to work with JavaScript Object Notation (JSON) in managed code.  
+
+> [!IMPORTANT]
+> For .NET Framework 4.6.1 and later versions, the APIs in the <xref:System.Text.Json> namespace should be used for serialization and deserialization. For earlier versions of .NET Framework, use [Newtonsoft.Json](https://www.newtonsoft.com/json).


This should be .NET Framework 4.7.2 or later. Even though S.T.Json is ns2.0 compatible, 4.6.1 support has an asterisk/footnote, so I think if we are going to recommend something, let's be a bit conservative and suggest 4.7.2 instead.

https://docs.microsoft.com/en-us/dotnet/standard/net-standard#net-implementation-support

2 The versions listed here represent the rules that NuGet uses to determine whether a given .NET Standard library is applicable. While NuGet considers .NET Framework 4.6.1 as supporting .NET Standard 1.5 through 2.0, there are several issues with consuming .NET Standard libraries that were built for those versions from .NET Framework 4.6.1 projects. For .NET Framework projects that need to use such libraries, we recommend that you upgrade the project to target .NET Framework 4.7.2 or higher.

ahsonkhan · 2020-03-25T01:15:42Z

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

    <remarks>
      <format type="text/markdown"><![CDATA[  

 ## Remarks  
- The <xref:System.Web.Script.Serialization.JavaScriptSerializer> class is used internally by the asynchronous communication layer to serialize and deserialize the data that is passed between the browser and the Web server. You cannot access that instance of the serializer. However, this class exposes a public API. Therefore, you can use the class when you want to work with JavaScript Object Notation (JSON) in managed code.  
+
+> [!IMPORTANT]


Can you please share the internal docs link so I can take a quick glance at how this will end up looking like on the docs page.

@ahsonkhan When the CI finishes, click on "Details", then scroll down to the preview link for this API.

I don't see it:
https://opbuildstorageprod.blob.core.windows.net/report/2020%5C3%5C25%5C500c749d-de78-3d4a-00e4-269d599bf9ec%5CPullRequest%5C202003250031355427-3758%5Cworkflow_report.html?sv=2016-05-31&sr=b&sig=NFFwGrzp7innzIOjl3ukNRilz7HD1L0hOknTawYzb98%3D&st=2020-03-25T01%3A43%3A15Z&se=2020-04-25T01%3A48%3A15Z&sp=r

OK, I got it. I took one of the other links and just navigated to the JavaScriptSerializer type:
https://review.docs.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer?view=netframework-4.8&branch=pr-en-us-3758

Why does it say that this page isn't available for netfx 4.8?
https://review.docs.microsoft.com/en-us/dotnet/api/system.text.json?view=netcore-3.1&branch=pr-en-us-3758&viewFallbackFrom=netframework-4.8

That could confuse users when we just said that for netfx 4.7.2+ use S.T.J. Is the platform support matrix in the docs incomplete? I would have expected anything that is netstanard2.0 compatible to have an entry in the appropriate netfx versions of the page (rather than redirecting to the .NET core version).

I don't see it
That's odd. But I'm glad you found it by navigating into another API.
Why does it say that this page isn't available for netfx 4.8?
It's also not showing up in NetStandard2.0. Do you see the same problem in the production MS Docs site?

Do you see the same problem in the production MS Docs site?

Yes.

@joelmartinez can you help us figure out why System.Text.Json is not showing up in the expected monikers?

I think @mimisasouvanh might be better point of contact here.

ahsonkhan · 2020-03-25T01:17:50Z

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

@@ -16,14 +16,19 @@
  <Interfaces />
  <Docs>
    <summary>
-      <see href="https://www.newtonsoft.com/json">Json.NET</see> should be used for serialization and deserialization. Provides serialization and deserialization functionality for AJAX-enabled applications.</summary>
+      Provides serialization and deserialization functionality for AJAX-enabled applications.


If we are truly recommending against this in favor of Newontosft.Json for < netfx 4.7.2, should we consider obsoleting the API in .NET Core going forward or is there no need for that/the usage too high? Of course for net4.7.2+ and .net core S.T.Json is the way to go :)

cc @terrajobst, @GrabYourPitchforks

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

Co-Authored-By: Ahson Khan <[email protected]>

Change Json.NET to System.Text.Json

1bcc5a5

tdykstra requested a review from scottaddie as a code owner January 10, 2020 03:56

dotnet-bot added this to the January 2020 milestone Jan 10, 2020

scottaddie reviewed Jan 10, 2020

View reviewed changes

mairaw requested a review from ahsonkhan January 28, 2020 08:24

BillWagner modified the milestones: January 2020, March 2020 Mar 2, 2020

carlossanlop requested review from steveharter, layomia and jozkee March 23, 2020 20:55

tdykstra added 4 commits March 24, 2020 17:28

move serializer advice to remarks

4630528

fix indentation

01bfb71

fix typo

eab8044

fix link

5a47156

ahsonkhan reviewed Mar 25, 2020

View reviewed changes

tdykstra commented Mar 25, 2020

View reviewed changes

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml Outdated Show resolved Hide resolved

tdykstra added 2 commits March 25, 2020 09:15

4.6.1 --> 4.7.2

5768f70

include use s.t.j/newtonsoft advice in summary

5a1d082

tdykstra commented Mar 25, 2020

View reviewed changes

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml Outdated Show resolved Hide resolved

fix typo

7324933

ahsonkhan reviewed Mar 25, 2020

View reviewed changes

xml/System.Web.Script.Serialization/JavaScriptSerializer.xml Outdated Show resolved Hide resolved

ahsonkhan approved these changes Mar 25, 2020

View reviewed changes

Update xml/System.Web.Script.Serialization/JavaScriptSerializer.xml

4350324

Co-Authored-By: Ahson Khan <[email protected]>

tdykstra merged commit 120ac05 into master Mar 25, 2020

tdykstra deleted the tdykstra-patch-2 branch March 25, 2020 21:39

Rageking8 mentioned this pull request Apr 3, 2024

Very odd English #1658

Open

Change Json.NET to System.Text.Json #3758

Change Json.NET to System.Text.Json #3758

Uh oh!

Conversation

tdykstra commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tdykstra commented Jan 10, 2020 •

edited

Loading